Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP List JRules inside openhab web interface #90

Merged
merged 9 commits into from
Mar 29, 2023

Conversation

seime
Copy link
Collaborator

@seime seime commented Nov 13, 2022

Make rules visible in openHAB and provide easy enabling/disabling

@seime seime marked this pull request as draft November 13, 2022 19:55
@seime seime mentioned this pull request Nov 13, 2022
@seaside1
Copy link
Owner

Rebased and fixed to be up to date with main

@seime
Copy link
Collaborator Author

seime commented Mar 13, 2023

@querdenker2k do you see why some of the ITs are failing?

@querdenker2k
Copy link
Collaborator

It's because all rules are disabled by default and they are not triggered.
Adding rule: org.openhab.automation.jrule.rules.user.TestPersistence@421cba65, enabled: false

@seime seime marked this pull request as ready for review March 21, 2023 19:42
@seaside1
Copy link
Owner

Nice. I'll try to review shortly.

@seaside1
Copy link
Owner

seaside1 commented Mar 22, 2023

Some comments so far, just tested it. I really like this a lot!

  • Would it be better to write out what rules are disabled ? Now it is a warning saying "Note X rules are disabled")
  • Should we add an annotation to disable a rule?
  • Enable / Disable seems to work
  • Run now on rule does not seem to work (also mentioned in documentation so it's ok but would be a cool feature)
  • JRuleLoadingStatistics could possibly be reset, cleared instead of creating a new instance to follow similar pattern as ruleprovider etc
  • Tests are failing for me, not sure if it is an intermittent problem that is not caused by this PR.

@seime
Copy link
Collaborator Author

seime commented Mar 23, 2023

  1. Writing out disabled rules; agreed
  2. Yes. Also been thinking about the ability to programmatically enable/disable rules, but have not made up my mind if this is an anti-pattern that instead should be resolved with preconditions
  3. Correct, it would require generating config for input data. Initially I tried to provide more info about the triggers, but did not figure out how to do this unless using existing predefined concepts that would not always match JRule
  4. Yes
  5. They are running on my side atm, but I think we have a timing/concurrency issue in general that should be resolved. When I reboot openHAB rules are compiled and loaded from 1-3 times. We might need to introduce the ReadyService mechanism or change the osgi start level - or something totally different. Have not investigated.

Copy link
Owner

@seaside1 seaside1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

@seaside1 seaside1 merged commit e79c9b6 into seaside1:main Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants